Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix repeat sequence double running bug #33

Merged
merged 9 commits into from
Feb 6, 2024

Conversation

kaphula
Copy link
Collaborator

@kaphula kaphula commented Jan 27, 2024

There's a bug in repeat sequence behavior where if its sequence list's first behavior returns Running, then when the behavior tree is ticked again, the condition behavior of repeat sequence is run again even though it should not be allowed when the behavior tree is in the middle of executing its sequence.

This PR fixes that, adds some tests and additional documentation.

@kaphula kaphula marked this pull request as draft January 28, 2024 06:47
@kaphula
Copy link
Collaborator Author

kaphula commented Jan 28, 2024

Actually I want to investigate and test this a bit more since it fixes some parts from my own application but break others.

@kaphula
Copy link
Collaborator Author

kaphula commented Jan 29, 2024

I think I got it now but let's keep it in draft for few days just to be safe. I get some time to see it in action and catch any funky behavior in case there's still something that is broken.

@kaphula
Copy link
Collaborator Author

kaphula commented Jan 31, 2024

I have not noticed anything weird and my code is now behaving as expected with this PR.

@kaphula kaphula marked this pull request as ready for review January 31, 2024 01:17
@Sollimann
Copy link
Owner

Will find some time in the next couple of days to review it👌

Copy link
Collaborator

@kembofly kembofly left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great job with the thorough investigation and testing!

@kaphula
Copy link
Collaborator Author

kaphula commented Feb 6, 2024

Thanks! We should probably add similar code examples to other behavior comment sections as seen in this PR. I can look into it at least for some of them at some point. I have also new behavior implemented called Fallback. It is very simple, basically an If behavior without the fail branch. I'll make a PR at some point so we can discuss if it makes sense to get included.

@kaphula kaphula merged commit 4b069c4 into main Feb 6, 2024
2 checks passed
@Sollimann
Copy link
Owner

@kaphula Wrt to the Fallback node. My preference is to keep the number of behaviors to a minimum unless they are strictly needed, and cannot be constructed with the existing behaviors already. The typical Fallback node corresponds to the Select behavior already.

/// Runs behaviors one by one until a behavior succeeds.
///
/// If a behavior fails it will try the next one.
/// Fails if the last behavior fails.
/// Can be thought of as a short-circuited logical OR gate.
Select(Vec<Behavior<A>>),

This behavior iterates over its children, executing them one at a time. If a child behavior results in a Failure, it moves on to the next child. The Select behavior succeeds as soon as one of its children succeeds. If all the children fail, then the Select behavior itself fails, exactly as a Fallback node would.

you can compare with Fallback node in the defacto c++ lib for behavior trees: https://www.behaviortree.dev/docs/nodes-library/fallbacknode/

@kaphula
Copy link
Collaborator Author

kaphula commented Feb 8, 2024

@kaphula Wrt to the Fallback node. My preference is to keep the number of behaviors to a minimum unless they are strictly needed, and cannot be constructed with the existing behaviors already. The typical Fallback node corresponds to the Select behavior already.

I agree with the reasoning of don't bloat for the sake of it. I am currently using this kind of extension to emulate the fallback behavior which works just fine for me:

pub fn fallback_bonsai_behavior<A: Clone>(from: Box<Behavior<A>>, to: Box<Behavior<A>>) -> Behavior<A> {
    If(from, to.clone(), to)
}

But now that I think of it, Select probably does serve the exact same purpose as fallback if you just declare the last behavior of Select as your fallback behavior. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants